-
Notifications
You must be signed in to change notification settings - Fork 89
Normalize file paths to Unix format in compiled .lock.yml files #13853
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot recompile |
|
@copilot make sure the paths passed in runtime-import is using ToSlash. |
Recompilation completed successfully. All 145 workflows compiled with no changes to existing |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Added |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func TestCompilerGeneratesUnixPaths(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| markdownContent string | ||
| expectedImportPaths []string | ||
| expectedIncludePaths []string | ||
| expectedSourcePath string | ||
| }{ | ||
| { | ||
| name: "imports with forward slashes", | ||
| markdownContent: `--- | ||
| on: issues | ||
| imports: | ||
| - shared/common.md | ||
| - shared/reporting.md | ||
| source: workflows/test-workflow.md | ||
| --- | ||
|
|
||
| # Test Workflow | ||
|
|
||
| This is a test workflow with imports.`, | ||
| expectedImportPaths: []string{ | ||
| "shared/common.md", | ||
| "shared/reporting.md", | ||
| }, | ||
| expectedSourcePath: "workflows/test-workflow.md", | ||
| }, | ||
| { | ||
| name: "includes with forward slashes", | ||
| markdownContent: `--- | ||
| on: pull_request | ||
| --- | ||
|
|
||
| # Test Include Workflow | ||
|
|
||
| {{#import shared/tools.md}} | ||
|
|
||
| This workflow includes external tools.`, | ||
| expectedIncludePaths: []string{ | ||
| "shared/tools.md", | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| // Create temporary directory for test | ||
| tmpDir := t.TempDir() | ||
|
|
||
| // Create shared directory and files for imports/includes | ||
| sharedDir := filepath.Join(tmpDir, "shared") | ||
| err := os.MkdirAll(sharedDir, 0755) | ||
| require.NoError(t, err, "Failed to create shared directory") | ||
|
|
||
| // Create shared/common.md (shared workflow - minimal valid content) | ||
| commonContent := `# Common Shared Workflow | ||
|
|
||
| This is a shared workflow.` | ||
| commonFile := filepath.Join(sharedDir, "common.md") | ||
| err = os.WriteFile(commonFile, []byte(commonContent), 0644) | ||
| require.NoError(t, err, "Failed to create common.md") | ||
|
|
||
| // Create shared/reporting.md (shared workflow - minimal valid content) | ||
| reportingContent := `# Reporting Shared Workflow | ||
|
|
||
| This is a shared workflow.` | ||
| reportingFile := filepath.Join(sharedDir, "reporting.md") | ||
| err = os.WriteFile(reportingFile, []byte(reportingContent), 0644) | ||
| require.NoError(t, err, "Failed to create reporting.md") | ||
|
|
||
| // Create shared/tools.md (shared workflow - minimal valid content) | ||
| toolsContent := `# Tools Shared Workflow | ||
|
|
||
| This is a shared workflow.` | ||
| toolsFile := filepath.Join(sharedDir, "tools.md") | ||
| err = os.WriteFile(toolsFile, []byte(toolsContent), 0644) | ||
| require.NoError(t, err, "Failed to create tools.md") | ||
|
|
||
| // Create workflows directory for source path | ||
| workflowsDir := filepath.Join(tmpDir, "workflows") | ||
| err = os.MkdirAll(workflowsDir, 0755) | ||
| require.NoError(t, err, "Failed to create workflows directory") | ||
|
|
||
| // Write markdown file | ||
| markdownPath := filepath.Join(tmpDir, "test-workflow.md") | ||
| err = os.WriteFile(markdownPath, []byte(tt.markdownContent), 0644) | ||
| require.NoError(t, err, "Failed to write markdown file") | ||
|
|
||
| // Compile the workflow | ||
| compiler := NewCompiler() | ||
| err = compiler.CompileWorkflow(markdownPath) | ||
| require.NoError(t, err, "Compilation should succeed") | ||
|
|
||
| // Read the generated .lock.yml file | ||
| lockFile := filepath.Join(tmpDir, "test-workflow.lock.yml") | ||
| lockContent, err := os.ReadFile(lockFile) | ||
| require.NoError(t, err, "Failed to read lock file") | ||
|
|
||
| lockYAML := string(lockContent) | ||
|
|
||
| // Verify that file paths in the manifest use forward slashes (Unix-compatible) | ||
| // Note: The ASCII art header contains backslashes, so we only check the manifest section | ||
| manifestStart := strings.Index(lockYAML, "# Resolved workflow manifest:") | ||
| sourceStart := strings.Index(lockYAML, "# Source:") | ||
|
|
||
| // Verify expected import paths are present with forward slashes | ||
| for _, importPath := range tt.expectedImportPaths { | ||
| expectedLine := "# - " + importPath | ||
| assert.Contains(t, lockYAML, expectedLine, "Lock file should contain import path: %s", importPath) | ||
|
|
||
| // Ensure no backslash version exists | ||
| backslashPath := strings.ReplaceAll(importPath, "/", "\\") | ||
| backslashLine := "# - " + backslashPath | ||
| assert.NotContains(t, lockYAML, backslashLine, "Lock file should not contain backslash version of: %s", importPath) | ||
| } | ||
|
|
||
| // Verify expected include paths are present with forward slashes | ||
| for _, includePath := range tt.expectedIncludePaths { | ||
| expectedLine := "# - " + includePath | ||
| assert.Contains(t, lockYAML, expectedLine, "Lock file should contain include path: %s", includePath) | ||
|
|
||
| // Ensure no backslash version exists | ||
| backslashPath := strings.ReplaceAll(includePath, "/", "\\") | ||
| backslashLine := "# - " + backslashPath | ||
| assert.NotContains(t, lockYAML, backslashLine, "Lock file should not contain backslash version of: %s", includePath) | ||
| } | ||
|
|
||
| // Verify source path uses forward slashes | ||
| if tt.expectedSourcePath != "" { | ||
| expectedLine := "# Source: " + tt.expectedSourcePath | ||
| assert.Contains(t, lockYAML, expectedLine, "Lock file should contain source path: %s", tt.expectedSourcePath) | ||
|
|
||
| // Ensure no backslash version exists | ||
| backslashPath := strings.ReplaceAll(tt.expectedSourcePath, "/", "\\") | ||
| backslashLine := "# Source: " + backslashPath | ||
| assert.NotContains(t, lockYAML, backslashLine, "Lock file should not contain backslash version of source: %s", tt.expectedSourcePath) | ||
| } | ||
|
|
||
| // Verify that manifest section does not contain backslashes in file paths | ||
| if manifestStart >= 0 { | ||
| manifestEnd := strings.Index(lockYAML[manifestStart:], "\n\n") | ||
| if manifestEnd >= 0 { | ||
| manifest := lockYAML[manifestStart : manifestStart+manifestEnd] | ||
| assert.NotContains(t, manifest, "\\", "Lock file manifest should not contain backslashes in file paths") | ||
| } | ||
| } | ||
|
|
||
| // Verify that source section does not contain backslashes in file paths | ||
| if sourceStart >= 0 && tt.expectedSourcePath != "" { | ||
| sourceEnd := strings.Index(lockYAML[sourceStart:], "\n") | ||
| if sourceEnd >= 0 { | ||
| sourceLine := lockYAML[sourceStart : sourceStart+sourceEnd] | ||
| // Check that the source line doesn't contain a Windows path | ||
| backslashPath := strings.ReplaceAll(tt.expectedSourcePath, "/", "\\") | ||
| assert.NotContains(t, sourceLine, backslashPath, "Source line should not contain Windows-style path") | ||
| } | ||
| } | ||
| }) | ||
| } | ||
| } |
Copilot
AI
Feb 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test suite doesn't verify the runtime-import path normalization. Consider adding a test case that checks the generated runtime-import macro path in the lock file contains forward slashes. For example, verify that a workflow file at workflows\test.md on Windows generates {{#runtime-import workflows/test.md}} instead of {{#runtime-import workflows\test.md}}.
The compiler generates platform-specific paths in
.lock.ymlfiles. On Windows, this produces backslash separators which breaks cross-platform workflows.Changes
Applied
filepath.ToSlash()normalization at all YAML output points:pkg/workflow/compiler_yaml.go: NormalizeSource,ImportedFiles,IncludedFiles, andruntime-importpaths before writing to YAML header and promptpkg/parser/include_expander.go: Normalize paths when building the included files manifestExample
Before (Windows):
After (all platforms):
Added test coverage in
compiler_path_normalization_test.gofor imports, includes, source paths, and nested directory structures.Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.